-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update charter.adoc per Greg's feedback #1
Conversation
Revise to clarify intent of existing extensions, and add justification for second extension Signed-off-by: Beeman Strong <[email protected]>
charter.adoc
Outdated
* An extension that enables precise attribution of samples based on select events (e.g., instruction/uop retirement events) to the instruction that caused the counter overflow, despite implementations where the associated sampling interrupt may skid. This will provide more directly actionable information to the user, by precisely identifying the instructions that are most often experiencing performance events. | ||
* An extension that enables sampling of instructions and/or uops, with collection of runtime metadata for the instruction/uop, including data virtual address, selecct event occurrences, and latencies incurred. Such samples can be filtered based on instruction/uop type, events incurred, or latencies observed, allowing the user to focus on samples of interest. Further, associated sampling interrupts can be skidless, allowing the user to collect additional sample state (call-stack, register values) reliably. | ||
|
||
Each extension will be crafted to be implementation-friendly even for high-performance, out-of-order microarchitectures, aiming to require no additional performance overhead beyond that resulting from the handling of sampling interrupts. The extensions will be compatible with the H extension, and support RISC-V security objectives. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to mention zero cost as well as zero overhead. That is, also clarify that implementing the extension should (even when disabled) should have zero additional performance cost?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the cost vs overhead distinction. But I wasn't thinking that we would aim to mandate no overhead, at least when the capability is enabled. Rather, my thinking was that we craft the ISA to ensure there is a viable path to an implementation that adds no overhead (besides interrupts) when the capability is enabled. But it will probably always be cheaper to implement with some slowdown, and I didn't think we wanted to forbid that.
I agree that, when the capability is not enabled, there should be no overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tweaked the language there, now says
...aiming to require no performance overhead when enabled beyond that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I think I missed the significance of the phrase "aiming to require" first time around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that seemed confusing in retrospect. I revised it again, have a look.
fix typo, clarify that overhead is only when enabled Signed-off-by: Beeman Strong <[email protected]>
Clarify that no performance overhead is a goal, not a requirement Signed-off-by: Beeman Strong <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Revise to clarify intent of existing extensions, and add justification for second extension